Couldn’t these 4 be made into a single interface with options for status? In other words, extending the old interface.

It could be merged, but I made this change on purpose. I wanted to use a state machine like approach (without introducing a dedicated lib like xstate) to make the code more easy to reason about.

The old interface looked simple, because it was just one interface, but it was (imho) harder to understand, because it was not very explicit. There was no differentiation between the initial state and the ready state for example, so there was the need to mock a proper result. But inside the component you never knew, if the result was mocked or not. (Also the user briefly saw "invalid" results, because of this.)

I could have added mocked results to the initial, manual and running states. They wouldn't "hurt", but they also wouldn't have any purpose. But it get's harder if you introduce new states which need other data (like error).

tl;dr: Yes, the interface became more verbose. But it also became more correct and explicit and I hope this will make it easier to understand and extend the logic of this component.

Long term, this addon should start using the useAddonState tbh.

@ndelangen Sorry, just saw your comment now. Did I understand you correctly that the approach is fine for now and useAddonState should be used in a future PR? Or should I add useAddonState to this PR?

Are we sure that the story has been rendered before calling this.request()? 🤷‍♂ I'm just asking because STORY_RENDERED event was used before

It seemed to work for me without problems, but I don't know the internals. Maybe @ndelangen could confirm if events emitted inside makeDecorator -> wrapper (see https://github.com/storybookjs/storybook/pull/8883/files/d73733d99e2250d840f35e16044e8de69bdc9b6c#diff-135e5fd1db00e7a987038977b6cb9148R70) happen after rendering? If not, do you know an example I could add to the tests to see when it would fail, so I can fix it?

STORY_RENDERED is called after the awaited render function resolved/returned:
https://github.com/storybookjs/storybook/blob/next/lib/core/src/client/preview/start.js#L240-L241

So I do believe it's called later. Whether or not this impacts the a11y execution, I don't know.

There actually seems to be a problem - not sure if it's because of the missing STORY_RENDERED or some other change I made. If another panel is initially active and you switch to the a11y panel you only see the "Initializing..." state.

E.g. if you visit https://monorepo-51csjv7sk.now.sh/cra-kitchen-sink/?path=/story/app--full-app for example (taken from a different pull request) and switch to the a11y you see results. While you only see the "Initializing..." state (at least on first try - if you reload it works) if you run my pull request locally.

I probably need to fix that 😭

Good catch @donaldpipowitch, yes we need to fix this, I will try to take a look tonight if I have the time ;)

@donaldpipowitch I needed to keep an Icon component with height and width otherwise the check icon from e368e57#diff-3250ce08c136d40dd47659131333a6a7R213 is not displayed

@donaldpipowitch I succeeded to fix locally the issue we talk about a few days ago (https://github.com/storybookjs/storybook/pull/8883/files?file-filters%5B%5D=.snap&file-filters%5B%5D=.ts&file-filters%5B%5D=.tsx#r364712456) by removing the if-check in the request function. Can you try on your side? and add a commit in your branch if it works?

request = () => {
  const { api } = this.props;
  this.setState(
    {
      status: 'running',
    },
    () => {
      api.emit(EVENTS.REQUEST);
      // removes all elements from the redux map in store from the previous panel
      store.dispatch(clearElements());
    }
  );
};

I don't well understand why this active check is done, I played with SB examples locally and didn't find any new issue 🤷‍♂

@gaetanmaisse thank you for having a look. This seems to fix the issue - it now works for me locally.

I'm unsure about active as well. I mostly left this untouched. But there is an old comment which says that it might can be removed completely?

For now I just added your fix and kept other usages of active.

I tried to simply remove all active usages and it breaks everything so I think we need to keep it for now.